Remove DependencyInner as a public API
authorAlex Crichton <alex@alexcrichton.com>
Sat, 3 Jun 2017 00:13:52 +0000 (17:13 -0700)
committerAlex Crichton <alex@alexcrichton.com>
Mon, 5 Jun 2017 14:36:44 +0000 (07:36 -0700)
No need for it to be exposed any more, let's just use `Rc::make_mut`
judiciously.

src/cargo/core/dependency.rs
src/cargo/core/mod.rs
src/cargo/core/registry.rs
src/cargo/ops/resolve.rs
src/cargo/sources/registry/mod.rs
src/cargo/util/toml.rs

index 4b363e6a70471d9c2162e9d0f760ce6d70814142..c748ee9c51189259c79ca9513ef51cd42ff58d84 100644 (file)
@@ -14,12 +14,12 @@ use util::errors::{CargoResult, CargoResultExt, CargoError};
 /// Cheap to copy.
 #[derive(PartialEq, Clone, Debug)]
 pub struct Dependency {
-    inner: Rc<DependencyInner>,
+    inner: Rc<Inner>,
 }
 
 /// The data underlying a Dependency.
 #[derive(PartialEq, Clone, Debug)]
-pub struct DependencyInner {
+struct Inner {
     name: String,
     source_id: SourceId,
     req: VersionReq,
@@ -79,6 +79,40 @@ pub enum Kind {
     Build,
 }
 
+fn parse_req_with_deprecated(req: &str,
+                             extra: Option<(&PackageId, &Config)>)
+                             -> CargoResult<VersionReq> {
+    match VersionReq::parse(req) {
+        Err(e) => {
+            let (inside, config) = match extra {
+                Some(pair) => pair,
+                None => return Err(e.into()),
+            };
+            match e {
+                ReqParseError::DeprecatedVersionRequirement(requirement) => {
+                    let msg = format!("\
+parsed version requirement `{}` is no longer valid
+
+Previous versions of Cargo accepted this malformed requirement,
+but it is being deprecated. This was found when parsing the manifest
+of {} {}, and the correct version requirement is `{}`.
+
+This will soon become a hard error, so it's either recommended to
+update to a fixed version or contact the upstream maintainer about
+this warning.
+",
+req, inside.name(), inside.version(), requirement);
+                    config.shell().warn(&msg)?;
+
+                    Ok(requirement)
+                }
+                e => Err(e.into()),
+            }
+        },
+        Ok(v) => Ok(v),
+    }
+}
+
 impl ser::Serialize for Kind {
     fn serialize<S>(&self, s: S) -> Result<S::Ok, S::Error>
         where S: ser::Serializer,
@@ -91,252 +125,178 @@ impl ser::Serialize for Kind {
     }
 }
 
-impl DependencyInner {
+impl Dependency {
     /// Attempt to create a `Dependency` from an entry in the manifest.
-    ///
-    /// The `deprecated_extra` information is set to `Some` when this method
-    /// should *also* parse historically deprecated semver requirements. This
-    /// information allows providing diagnostic information about what's going
-    /// on.
-    ///
-    /// If set to `None`, then historically deprecated semver requirements will
-    /// all be rejected.
     pub fn parse(name: &str,
                  version: Option<&str>,
                  source_id: &SourceId,
-                 deprecated_extra: Option<(&PackageId, &Config)>)
-                 -> CargoResult<DependencyInner> {
+                 inside: &PackageId,
+                 config: &Config) -> CargoResult<Dependency> {
+        let arg = Some((inside, config));
         let (specified_req, version_req) = match version {
-            Some(v) => (true, DependencyInner::parse_with_deprecated(v, deprecated_extra)?),
+            Some(v) => (true, parse_req_with_deprecated(v, arg)?),
             None => (false, VersionReq::any())
         };
 
-        Ok(DependencyInner {
-            only_match_name: false,
-            req: version_req,
-            specified_req: specified_req,
-            .. DependencyInner::new_override(name, source_id)
-        })
+        let mut ret = Dependency::new_override(name, source_id);
+        {
+            let ptr = Rc::make_mut(&mut ret.inner);
+            ptr.only_match_name = false;
+            ptr.req = version_req;
+            ptr.specified_req = specified_req;
+        }
+        Ok(ret)
     }
 
-    fn parse_with_deprecated(req: &str,
-                             extra: Option<(&PackageId, &Config)>)
-                             -> CargoResult<VersionReq> {
-        match VersionReq::parse(req) {
-            Err(e) => {
-                let (inside, config) = match extra {
-                    Some(pair) => pair,
-                    None => return Err(e.into()),
-                };
-                match e {
-                    ReqParseError::DeprecatedVersionRequirement(requirement) => {
-                        let msg = format!("\
-parsed version requirement `{}` is no longer valid
-
-Previous versions of Cargo accepted this malformed requirement,
-but it is being deprecated. This was found when parsing the manifest
-of {} {}, and the correct version requirement is `{}`.
-
-This will soon become a hard error, so it's either recommended to
-update to a fixed version or contact the upstream maintainer about
-this warning.
-",
-    req, inside.name(), inside.version(), requirement);
-                        config.shell().warn(&msg)?;
+    /// Attempt to create a `Dependency` from an entry in the manifest.
+    pub fn parse_no_deprecated(name: &str,
+                               version: Option<&str>,
+                               source_id: &SourceId) -> CargoResult<Dependency> {
+        let (specified_req, version_req) = match version {
+            Some(v) => (true, parse_req_with_deprecated(v, None)?),
+            None => (false, VersionReq::any())
+        };
 
-                        Ok(requirement)
-                    }
-                    e => Err(e.into()),
-                }
-            },
-            Ok(v) => Ok(v),
+        let mut ret = Dependency::new_override(name, source_id);
+        {
+            let ptr = Rc::make_mut(&mut ret.inner);
+            ptr.only_match_name = false;
+            ptr.req = version_req;
+            ptr.specified_req = specified_req;
         }
+        Ok(ret)
     }
 
-    pub fn new_override(name: &str, source_id: &SourceId) -> DependencyInner {
-        DependencyInner {
-            name: name.to_string(),
-            source_id: source_id.clone(),
-            req: VersionReq::any(),
-            kind: Kind::Normal,
-            only_match_name: true,
-            optional: false,
-            features: Vec::new(),
-            default_features: true,
-            specified_req: false,
-            platform: None,
+    pub fn new_override(name: &str, source_id: &SourceId) -> Dependency {
+        Dependency {
+            inner: Rc::new(Inner {
+                name: name.to_string(),
+                source_id: source_id.clone(),
+                req: VersionReq::any(),
+                kind: Kind::Normal,
+                only_match_name: true,
+                optional: false,
+                features: Vec::new(),
+                default_features: true,
+                specified_req: false,
+                platform: None,
+            }),
         }
     }
 
-    pub fn version_req(&self) -> &VersionReq { &self.req }
-    pub fn name(&self) -> &str { &self.name }
-    pub fn source_id(&self) -> &SourceId { &self.source_id }
-    pub fn kind(&self) -> Kind { self.kind }
-    pub fn specified_req(&self) -> bool { self.specified_req }
+    pub fn version_req(&self) -> &VersionReq {
+        &self.inner.req
+    }
 
-    /// If none, this dependency must be built for all platforms.
-    /// If some, it must only be built for matching platforms.
+    pub fn name(&self) -> &str {
+        &self.inner.name
+    }
+
+    pub fn source_id(&self) -> &SourceId {
+        &self.inner.source_id
+    }
+
+    pub fn kind(&self) -> Kind {
+        self.inner.kind
+    }
+
+    pub fn specified_req(&self) -> bool {
+        self.inner.specified_req
+    }
+
+    /// If none, this dependencies must be built for all platforms.
+    /// If some, it must only be built for the specified platform.
     pub fn platform(&self) -> Option<&Platform> {
-        self.platform.as_ref()
+        self.inner.platform.as_ref()
     }
 
-    pub fn set_kind(&mut self, kind: Kind) -> &mut DependencyInner {
-        self.kind = kind;
+    pub fn set_kind(&mut self, kind: Kind) -> &mut Dependency {
+        Rc::make_mut(&mut self.inner).kind = kind;
         self
     }
 
     /// Sets the list of features requested for the package.
-    pub fn set_features(&mut self, features: Vec<String>) -> &mut DependencyInner {
-        self.features = features;
+    pub fn set_features(&mut self, features: Vec<String>) -> &mut Dependency {
+        Rc::make_mut(&mut self.inner).features = features;
         self
     }
 
     /// Sets whether the dependency requests default features of the package.
-    pub fn set_default_features(&mut self, default_features: bool) -> &mut DependencyInner {
-        self.default_features = default_features;
+    pub fn set_default_features(&mut self, default_features: bool) -> &mut Dependency {
+        Rc::make_mut(&mut self.inner).default_features = default_features;
         self
     }
 
     /// Sets whether the dependency is optional.
-    pub fn set_optional(&mut self, optional: bool) -> &mut DependencyInner {
-        self.optional = optional;
+    pub fn set_optional(&mut self, optional: bool) -> &mut Dependency {
+        Rc::make_mut(&mut self.inner).optional = optional;
         self
     }
 
     /// Set the source id for this dependency
-    pub fn set_source_id(&mut self, id: SourceId) -> &mut DependencyInner {
-        self.source_id = id;
+    pub fn set_source_id(&mut self, id: SourceId) -> &mut Dependency {
+        Rc::make_mut(&mut self.inner).source_id = id;
         self
     }
 
     /// Set the version requirement for this dependency
-    pub fn set_version_req(&mut self, req: VersionReq) -> &mut DependencyInner {
-        self.req = req;
+    pub fn set_version_req(&mut self, req: VersionReq) -> &mut Dependency {
+        Rc::make_mut(&mut self.inner).req = req;
         self
     }
 
-    pub fn set_platform(&mut self, platform: Option<Platform>)
-                        -> &mut DependencyInner {
-        self.platform = platform;
+    pub fn set_platform(&mut self, platform: Option<Platform>) -> &mut Dependency {
+        Rc::make_mut(&mut self.inner).platform = platform;
         self
     }
 
     /// Lock this dependency to depending on the specified package id
-    pub fn lock_to(&mut self, id: &PackageId) -> &mut DependencyInner {
-        assert_eq!(self.source_id, *id.source_id());
-        assert!(self.req.matches(id.version()));
+    pub fn lock_to(&mut self, id: &PackageId) -> &mut Dependency {
+        assert_eq!(self.inner.source_id, *id.source_id());
+        assert!(self.inner.req.matches(id.version()));
         self.set_version_req(VersionReq::exact(id.version()))
             .set_source_id(id.source_id().clone())
     }
 
+
     /// Returns false if the dependency is only used to build the local package.
     pub fn is_transitive(&self) -> bool {
-        match self.kind {
+        match self.inner.kind {
             Kind::Normal | Kind::Build => true,
             Kind::Development => false,
         }
     }
-    pub fn is_build(&self) -> bool {
-        match self.kind { Kind::Build => true, _ => false }
-    }
-    pub fn is_optional(&self) -> bool { self.optional }
-    /// Returns true if the default features of the dependency are requested.
-    pub fn uses_default_features(&self) -> bool { self.default_features }
-    /// Returns the list of features that are requested by the dependency.
-    pub fn features(&self) -> &[String] { &self.features }
-
-    /// Returns true if the package (`sum`) can fulfill this dependency request.
-    pub fn matches(&self, sum: &Summary) -> bool {
-        self.matches_id(sum.package_id())
-    }
-
-    /// Returns true if the package (`id`) can fulfill this dependency request.
-    pub fn matches_id(&self, id: &PackageId) -> bool {
-        self.name == id.name() &&
-            (self.only_match_name || (self.req.matches(id.version()) &&
-                                      &self.source_id == id.source_id()))
-    }
-
-    pub fn into_dependency(self) -> Dependency {
-        Dependency {inner: Rc::new(self)}
-    }
-}
-
-impl Dependency {
-    /// Attempt to create a `Dependency` from an entry in the manifest.
-    pub fn parse(name: &str,
-                 version: Option<&str>,
-                 source_id: &SourceId,
-                 inside: &PackageId,
-                 config: &Config) -> CargoResult<Dependency> {
-        let arg = Some((inside, config));
-        DependencyInner::parse(name, version, source_id, arg).map(|di| {
-            di.into_dependency()
-        })
-    }
-
-    /// Attempt to create a `Dependency` from an entry in the manifest.
-    pub fn parse_no_deprecated(name: &str,
-                               version: Option<&str>,
-                               source_id: &SourceId) -> CargoResult<Dependency> {
-        DependencyInner::parse(name, version, source_id, None).map(|di| {
-            di.into_dependency()
-        })
-    }
-
-    pub fn new_override(name: &str, source_id: &SourceId) -> Dependency {
-        DependencyInner::new_override(name, source_id).into_dependency()
-    }
-
-    pub fn clone_inner(&self) -> DependencyInner { (*self.inner).clone() }
 
-    pub fn version_req(&self) -> &VersionReq { self.inner.version_req() }
-    pub fn name(&self) -> &str { self.inner.name() }
-    pub fn source_id(&self) -> &SourceId { self.inner.source_id() }
-    pub fn kind(&self) -> Kind { self.inner.kind() }
-    pub fn specified_req(&self) -> bool { self.inner.specified_req() }
-
-    /// If none, this dependencies must be built for all platforms.
-    /// If some, it must only be built for the specified platform.
-    pub fn platform(&self) -> Option<&Platform> {
-        self.inner.platform()
-    }
-
-    /// Lock this dependency to depending on the specified package id
-    pub fn lock_to(mut self, id: &PackageId) -> Dependency {
-        Rc::make_mut(&mut self.inner).lock_to(id);
-        self
-    }
-
-    /// Set the version requirement for this dependency
-    pub fn set_version_req(&mut self, req: VersionReq) -> &mut Dependency {
-        Rc::make_mut(&mut self.inner).set_version_req(req);
-        self
+    pub fn is_build(&self) -> bool {
+        match self.inner.kind {
+            Kind::Build => true,
+            _ => false,
+        }
     }
 
-    pub fn set_kind(&mut self, kind: Kind) -> &mut Dependency {
-        Rc::make_mut(&mut self.inner).set_kind(kind);
-        self
+    pub fn is_optional(&self) -> bool {
+        self.inner.optional
     }
 
-    /// Returns false if the dependency is only used to build the local package.
-    pub fn is_transitive(&self) -> bool { self.inner.is_transitive() }
-    pub fn is_build(&self) -> bool { self.inner.is_build() }
-    pub fn is_optional(&self) -> bool { self.inner.is_optional() }
-
     /// Returns true if the default features of the dependency are requested.
     pub fn uses_default_features(&self) -> bool {
-        self.inner.uses_default_features()
+        self.inner.default_features
     }
     /// Returns the list of features that are requested by the dependency.
-    pub fn features(&self) -> &[String] { self.inner.features() }
+    pub fn features(&self) -> &[String] {
+        &self.inner.features
+    }
 
     /// Returns true if the package (`sum`) can fulfill this dependency request.
-    pub fn matches(&self, sum: &Summary) -> bool { self.inner.matches(sum) }
+    pub fn matches(&self, sum: &Summary) -> bool {
+        self.matches_id(sum.package_id())
+    }
 
     /// Returns true if the package (`id`) can fulfill this dependency request.
     pub fn matches_id(&self, id: &PackageId) -> bool {
-        self.inner.matches_id(id)
+        self.inner.name == id.name() &&
+            (self.inner.only_match_name || (self.inner.req.matches(id.version()) &&
+                                      &self.inner.source_id == id.source_id()))
     }
 
     pub fn map_source(mut self, to_replace: &SourceId, replace_with: &SourceId)
@@ -344,8 +304,7 @@ impl Dependency {
         if self.source_id() != to_replace {
             self
         } else {
-            Rc::make_mut(&mut self.inner)
-                .set_source_id(replace_with.clone());
+            self.set_source_id(replace_with.clone());
             self
         }
     }
index e663dcdf27002ab267655133849cb9db7985ba65..2c966f681b41ff6949d00d7bf8810abfff27a643 100644 (file)
@@ -1,4 +1,4 @@
-pub use self::dependency::{Dependency, DependencyInner};
+pub use self::dependency::Dependency;
 pub use self::manifest::{Manifest, Target, TargetKind, Profile, LibKind, Profiles};
 pub use self::manifest::{EitherManifest, VirtualManifest};
 pub use self::package::{Package, PackageSet};
index 6c79620493b37bebfd877a3a89319b41a93c3db4..7c5292604a05b665e26fd8b0cf49ead7acd21d4d 100644 (file)
@@ -259,7 +259,7 @@ impl<'cfg> PackageRegistry<'cfg> {
             Some(&(ref precise, _)) => summary.override_id(precise.clone()),
             None => summary,
         };
-        summary.map_dependencies(|dep| {
+        summary.map_dependencies(|mut dep| {
             trace!("\t{}/{}/{}", dep.name(), dep.version_req(),
                    dep.source_id());
 
@@ -286,7 +286,8 @@ impl<'cfg> PackageRegistry<'cfg> {
                 let locked = locked_deps.iter().find(|id| dep.matches_id(id));
                 if let Some(locked) = locked {
                     trace!("\tfirst hit on {}", locked);
-                    return dep.lock_to(locked)
+                    dep.lock_to(locked);
+                    return dep
                 }
             }
 
@@ -301,7 +302,8 @@ impl<'cfg> PackageRegistry<'cfg> {
             match v {
                 Some(&(ref id, _)) => {
                     trace!("\tsecond hit on {}", id);
-                    dep.lock_to(id)
+                    dep.lock_to(id);
+                    return dep
                 }
                 None => {
                     trace!("\tremaining unlocked");
index 8741c74259c4a315c24f10b5eed92e2b10a2286f..8a84071b42af1b06ae70a63226762bae31f6abfc 100644 (file)
@@ -217,7 +217,9 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry,
                     if spec.matches(key) &&
                        dep.matches_id(val) &&
                        keep(&val, to_avoid, &to_avoid_sources) {
-                        return (spec.clone(), dep.clone().lock_to(val))
+                        let mut dep = dep.clone();
+                        dep.lock_to(val);
+                        return (spec.clone(), dep)
                     }
                 }
                 (spec.clone(), dep.clone())
index 84f801ed8e570ceb2c09181bb7b5fabec54a95a5..beb768759a70c6eee0aacf2e470cf14f483f6352 100644 (file)
@@ -170,7 +170,7 @@ use serde::de;
 use tar::Archive;
 
 use core::{Source, SourceId, PackageId, Package, Summary, Registry};
-use core::dependency::{Dependency, DependencyInner, Kind};
+use core::dependency::{Dependency, Kind};
 use sources::PathSource;
 use util::{CargoResult, Config, internal, FileLock, Filesystem};
 use util::errors::CargoResultExt;
@@ -459,7 +459,7 @@ fn parse_registry_dependency(dep: RegistryDependency)
     } = dep;
 
     let mut dep = DEFAULT_ID.with(|id| {
-        DependencyInner::parse(&name, Some(&req), id, None)
+        Dependency::parse_no_deprecated(&name, Some(&req), id)
     })?;
     let kind = match kind.as_ref().map(|s| &s[..]).unwrap_or("") {
         "dev" => Kind::Development,
@@ -484,5 +484,5 @@ fn parse_registry_dependency(dep: RegistryDependency)
        .set_features(features)
        .set_platform(platform)
        .set_kind(kind);
-    Ok(dep.into_dependency())
+    Ok(dep)
 }
index b665d322dfeff18369399922c9ade2c0dc3b62e1..57fe72abb5a63c5b177e03bf46b87a0b13c61d17 100644 (file)
@@ -12,7 +12,7 @@ use serde::de::{self, Deserialize};
 use serde_ignored;
 
 use core::{SourceId, Profiles, PackageIdSpec, GitReference, WorkspaceConfig};
-use core::{Summary, Manifest, Target, Dependency, DependencyInner, PackageId};
+use core::{Summary, Manifest, Target, Dependency, PackageId};
 use core::{EitherManifest, VirtualManifest};
 use core::dependency::{Kind, Platform};
 use core::manifest::{LibKind, Profile, ManifestMetadata};
@@ -1111,10 +1111,10 @@ impl TomlDependency {
         let version = details.version.as_ref().map(|v| &v[..]);
         let mut dep = match cx.pkgid {
             Some(id) => {
-                DependencyInner::parse(name, version, &new_source_id,
-                                            Some((id, cx.config)))?
+                Dependency::parse(name, version, &new_source_id,
+                                  id, cx.config)?
             }
-            None => DependencyInner::parse(name, version, &new_source_id, None)?,
+            None => Dependency::parse_no_deprecated(name, version, &new_source_id)?,
         };
         dep.set_features(details.features.unwrap_or(Vec::new()))
            .set_default_features(details.default_features
@@ -1125,7 +1125,7 @@ impl TomlDependency {
         if let Some(kind) = kind {
             dep.set_kind(kind);
         }
-        Ok(dep.into_dependency())
+        Ok(dep)
     }
 }